VRDP: Fix incorrect index check in QueryClientMonitorRect#542
VRDP: Fix incorrect index check in QueryClientMonitorRect#542ehdgks0627 wants to merge 1 commit intoVirtualBox:mainfrom
Conversation
QueryClientMonitorRect uses '>' instead of '>=' to validate uScreenId, allowing an out-of-range access to m_paRects when the server has more monitors than the client reported. The adjacent IsScreenMatched already uses '>=' correctly.
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
|
Thank you for signing the OCA. |
|
Thanks for the patch, but shouldn't this be uScreenId >= m_cMonitors (this is zero-based, so m_cMonitors - 1)? |
Description
QueryClientMonitorRect uses
>instead of>=to validateuScreenId, allowing an out-of-range access tom_paRectswhen the server has more monitors than the client reported. The adjacent IsScreenMatched already uses '>=' correctly.Analysis
m_paRectsis allocated withm_cMonitorselements (valid indices0tom_cMonitors - 1) inCalculateClientRect(line 662-686). WhenuScreenId == m_cMonitors, the guard passes andm_paRects[m_cMonitors]accesses past the array.virtualbox/src/VBox/RDP/server/vrdpdmap.cpp
Lines 691 to 703 in 354a83f